feat(api): add blend accrue keeper - #484
Conversation
|
@Adecom16 is attempting to deploy a commit to the Collins' projects Team on Vercel. A member of the Team first needs to authorize it. |
collinsezedike
left a comment
There was a problem hiding this comment.
CI is failing on Test, not for a failing test (all 117 pass) but on the coverage threshold: Coverage for branches (73.95%) does not meet global threshold (76%).
accrual-keeper.ts is at 64.04% branch coverage, with 120, 126, 131, 153, 191-198, 330-375, 396-399, 405-413 uncovered. That's the retry and failure-handling logic specifically, the part covering what happens when a submission fails, which is exactly what determines whether this keeper degrades safely or silently. Please add tests covering those branches so the coverage threshold passes.
Addressed the coverage feedback. Added targeted tests covering the keeper's retry, discovery, Blend filtering, submission, failure/recovery, and dependency paths. Branch coverage is now above the required CI threshold. |
collinsezedike
left a comment
There was a problem hiding this comment.
This is real, substantial infrastructure work, and the coverage fix addresses what CI flagged. But there are several unaddressed issues in code that signs and submits real transactions with a funded key, serious enough to hold this before merge.
api/v1/keepers/accrue.ts has no applyCors/checkRateLimit, unlike every other api/v1/* handler, despite each call potentially signing and submitting a real transaction.
The cron auth has a real bypass. isCronAuthorized returns true unconditionally whenever CRON_SECRET is unset and NODE_ENV !== "production". Vercel serverless functions don't reliably set NODE_ENV=production the way traditional Node apps do (Vercel's own environment signal is VERCEL_ENV), so this could be wide open on the actual production deployment, not just local dev. Please gate on VERCEL_ENV === "production" instead, and fail closed rather than open when the secret is unconfigured.
submitAccrualTransaction signs and submits against a live-discovered adapter address with no assertSubmittable allowlist check, unlike every other signed submission path in tx.ts. adapterId comes from get_adapter at run time; if a vault's stored adapter pointer is ever wrong, the keeper signs and submits against whatever address comes back with none of the known-contract checks assertSubmittable exists to enforce.
KNOWN_POOLS.mainnet has zero protocol: "meridian" entries, so discoverLiveAdapters's filter matches nothing there. The new mainnet cron in vercel.json will run every 15 minutes and silently no-op, returning 200 with empty results indistinguishable from a healthy run, while apps/docs/operations/accrual-keeper.md advertises a 15-minute staleness guarantee that isn't actually enforced for any mainnet vault yet.
Smaller things worth fixing in the same pass:
isTransientKeeperErrormatches unanchored substrings like"500"/"503"/"not_found", so a permanent error whose message happens to contain those digits gets retried instead of failing immediately.waitForTransactionis called withoutrpcTimeoutMs, soMERIDIAN_KEEPER_RPC_TIMEOUT_MS(documented as a real config knob) doesn't actually bound the confirmation-wait step.- The cron route in
vercel.jsonhas nofunctions.maxDuration, while the keeper processes adapters sequentially with retries and confirmation waits, that adds up as more vaults go live. submitAccrualTransactionandwithKeeperRetryreimplementtx.ts's submission pipeline andshared/utils.ts'swithRetryinstead of reusing them, so future fixes to either won't propagate here.
|
Hey @Adecom16, checking in on the review from a few days ago. The coverage fix was solid, but the second round flagged some real issues in code that signs and submits live transactions (the cron auth bypass, the missing rate limit, the missing assertSubmittable check) that need to be addressed before this can merge. Let me know if anything in there is unclear or if you're stuck on any of it. |
Closes #466
Summary
accrue()only for Blend-backed adapters.Validation